Remove persisted styles when calling cancel()#3656
Conversation
cancel() now removes inline styles that were persisted by Motion's style persistence mechanism, making the element revert to its pre-animation state. Previously, cancel() only cancelled the WAAPI animation but left the persisted inline style in place. Changes: - NativeAnimation.cancel(): removes persisted inline style via removeStyle() - NativeAnimation.stop(): calls native WAAPI cancel directly to preserve committed styles (stop should keep the current value) - JSAnimation.cancel(): resets state to "running" before tick(0) so the animation properly resets to its initial value when cancelled after finish - Added removeStyle() utility alongside existing setStyle() Fixes #2948 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR fixes Key changes:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant NativeAnimation
participant WAAPI as Browser WAAPI
participant DOM as Element Inline Style
Note over NativeAnimation,DOM: Normal finish flow
WAAPI->>NativeAnimation: onfinish callback
NativeAnimation->>DOM: setStyle(element, name, finalKeyframe)
NativeAnimation->>WAAPI: animation.cancel() [removes keyframe effect]
Note over User,DOM: cancel() after finish (NEW behavior)
User->>NativeAnimation: cancel()
NativeAnimation->>WAAPI: animation.cancel() [no-op, already cancelled]
NativeAnimation->>DOM: removeStyle(element, name) [clears persisted style ✓]
Note over User,DOM: stop() during animation (fixed)
User->>NativeAnimation: stop()
NativeAnimation->>NativeAnimation: commitStyles() / updateMotionValue()
NativeAnimation->>DOM: setStyle(element, name, currentValue)
NativeAnimation->>WAAPI: animation.cancel() [direct, NOT this.cancel()]
Note over DOM: Inline style preserved ✓
Note over User,DOM: cancel() mid-animation (potential regression)
User->>NativeAnimation: cancel() [before onfinish]
NativeAnimation->>WAAPI: animation.cancel() [removes keyframe effect]
NativeAnimation->>DOM: removeStyle(element, name) [⚠ may clear pre-existing style]
Last reviewed commit: 32a18a3 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const { element, name } = this.options || {} | ||
| if (element && name && !this.isPseudoElement) { | ||
| removeStyle(element, name) | ||
| } |
There was a problem hiding this comment.
removeStyle() called unconditionally on mid-animation cancel
removeStyle() is called here regardless of whether the animation has already finished. setStyle() is only invoked inside the onfinish handler (which also sets this.finishedTime). For a mid-animation cancel() call, setStyle() will never have run, so removeStyle() will inadvertently erase any pre-existing inline style that Motion itself did not write.
Consider this scenario:
element.style.opacity = '0' // pre-existing inline style
const anim = animate(element, { opacity: 1 }, { duration: 1 })
// 500ms in…
anim.cancel()
// Before this PR → opacity reverts to '0' (WAAPI removes its effect, inline '0' survives)
// After this PR → opacity becomes '' (removeStyle clears the pre-existing value)Because finishedTime !== null is already a reliable proxy for "the onfinish handler ran and setStyle() persisted the final value", the guard should include that check:
| const { element, name } = this.options || {} | |
| if (element && name && !this.isPseudoElement) { | |
| removeStyle(element, name) | |
| } | |
| const { element, name } = this.options || {} | |
| if (element && name && !this.isPseudoElement && this.finishedTime !== null) { | |
| removeStyle(element, name) | |
| } |
| const { element, name } = this.options || {} | ||
| if (element && name && !this.isPseudoElement) { | ||
| removeStyle(element, name) | ||
| } |
There was a problem hiding this comment.
NativeAnimationExtended.stop() also calls setStyle() — cancel interacts with it
NativeAnimationExtended.updateMotionValue() (called from stop() when a motionValue is present) independently calls setStyle(element, name, current) to commit the mid-animation value before the WAAPI animation is cancelled. If a caller holds on to a stopped NativeAnimationExtended and later calls cancel(), removeStyle() will silently erase that committed mid-animation value even though finishedTime is still null.
With the finishedTime !== null guard suggested above, those mid-stop committed styles would survive a subsequent cancel(), which matches the intent of stop(). However, consider whether a cancel() after a stop() should instead revert completely — and if so, whether a dedicated flag (e.g. private hasPersistedStyle) would be a cleaner, more explicit way to track whether setStyle() was called, regardless of the path.
| this.startTime = 0 | ||
| this.state = "running" | ||
| this.tick(0) | ||
| this.teardown() | ||
| this.options.onCancel?.() |
There was a problem hiding this comment.
teardown() / double decrement of activeAnimations.mainThread
finish() calls teardown() (which does activeAnimations.mainThread--), then sets this.state = "finished". If cancel() is subsequently called, it now produces observable effects (the tick(0) revert), so users are more likely to call it after finish than before. teardown() will run a second time, decrementing activeAnimations.mainThread again.
This is a pre-existing issue (the teardown() call was already in cancel() before this PR), but this change makes the double-decrement reachable in normal usage for the first time.
A simple guard would prevent it:
| this.startTime = 0 | |
| this.state = "running" | |
| this.tick(0) | |
| this.teardown() | |
| this.options.onCancel?.() | |
| this.state = "running" | |
| this.tick(0) | |
| if (this.state !== "idle") this.teardown() |
Or alternatively check activeAnimations.mainThread > 0 before decrementing inside teardown().
1 participant
Summary
cancel()now removes inline styles that were persisted by Motion's style persistence mechanism, reverting the element to its pre-animation statecancel()only cancelled the WAAPI animation but left the persisted inline style in place, making it impossible to undo an animation's visual effectJSAnimation.cancel()after finish, which previously left the element at the final value becausetick(0)was a no-op when the animation state was "finished"Changes
NativeAnimation.cancel(): Now callsremoveStyle()to clear the persisted inline style after cancelling the WAAPI animationNativeAnimation.stop(): Calls native WAAPIcancel()directly instead ofthis.cancel(), so committed styles are preserved during interruptionJSAnimation.cancel(): Resetsstateto"running"beforetick(0)so the animation properly resets to its initial keyframe value when cancelled after finishremoveStyle(): New utility instyle-set.tsthat mirrorssetStyle()— removes a style property from an element's inline stylesNote: This is a behavioral change. Previously
cancel()after finish was a no-op (the final value remained committed). Now it properly reverts the animation effect, matching WAAPI cancel semantics.Fixes #2948
Test plan
NativeAnimation.test.tscovering cancel removes styles, cancel removes CSS custom properties, and stop preserves committed stylesanimate-cancel-removes-stylestest passing on React 18 and React 19cancel() after finishtest to assert revert behavior — passes on Chromium and WebKit🤖 Generated with Claude Code